feat(api): accept `granularity` on /api/v1/vector-viz/search by cbcoutinho · Pull Request #1359 · cbcoutinho/nextcloud-mcp-server

cbcoutinho · 2026-08-19T20:45:56Z

POST /api/v1/vector-viz/search is the only endpoint the Astrolabe app's search page calls, and it did not accept granularity at all — it passed no value to the search algorithm and hardcoded chunk into its metrics. granularity="document" (one row per document rather than per passage) was therefore unreachable from the UI, even though /api/v1/search and nc_semantic_search both expose it.

That matters beyond a missing feature: ADR-034's relevance curves were fitted at document granularity, so the app page could not request the retrieval shape its own relevance numbers were calibrated on.

How it was found

A benchmark sweep over rerankers, chunk sizes and embedders produced clean results — and then an audit of which of those settings the Astrolabe UI can actually reach found the answer was one (limit, and only to 50). The numbers described configurations no user could request. granularity was the worst case, because the endpoint had no such parameter at all.

What changed

Brings the endpoint into line with its sibling on all four points:

	before	after
value read	not at all	parsed from the body
unknown value	n/a	400, not a silent downgrade to `chunk`
`document` + `semantic`	n/a	422 with the same payload `/api/v1/search` returns
reaches the algorithm	never	both the single-search and `doc_types` branches

Also threads through effective_pool_size(grouped=…) — the grouped prefetch is bounded by MAX_DOCUMENT_PREFETCH, so asking for more groups than it can fill makes Qdrant widen its grouping search and reorder the head before the reranker sees it — and makes search metrics report the granularity actually used rather than a hardcoded chunk.

Compatibility

Purely additive. Omitting the field behaves exactly as before, which is what every existing Astrolabe release does (test_default_granularity_is_chunk pins this). A client sending granularity to an older server is ignored rather than erroring, so there is no deployment-ordering requirement in either direction.

Test coverage

New tests/unit/api/test_vector_viz_granularity_api.py, 6 tests: default is chunk; the value reaches the algorithm on both the plain and doc_types branches (a parameter threaded on one branch only would be invisible to anyone filtering by type, which the UI does on every search); unknown value → 400 without touching the algorithm; document+semantic → 422 with the full payload asserted; chunk+semantic still allowed.

Verified end to end against a live login-flow stack with 60 indexed Deck cards — granularity=document returns 200 with results, and document+semantic returns the 422, which is what proves the server reads the value rather than accepting and ignoring it.

e2e + contract: this adds a request parameter to an existing /api/v1/* route rather than a new route or MCP tool. The provider pact covers only the public endpoints (/api/v1/status, /api/v1/vector-sync/status) — the authenticated surface including this one remains unverified pending the ADR-029 phase-4 Bearer-token/provider-state hook, which is a pre-existing gap tracked on Deck board 11, not one this PR introduces. Full-stack behaviour is covered by the manual verification above; there is no dedicated e2e marker in this repo.

3,609 unit tests pass; ruff, ruff format, ty clean.

Deck #1070. The Astrolabe side (sending fusion/granularity/min_relevance, plus a 422 passthrough fix) is a separate PR in that repo.

This PR was generated with the help of AI, and reviewed by a Human

This endpoint is the only one the Astrolabe app's search page calls, and it did not accept `granularity` at all — it passed no value to the search algorithm and hardcoded `chunk` into its metrics. So `granularity="document"` (one row per document rather than per passage) was unreachable from the UI, even though `/api/v1/search` and `nc_semantic_search` both expose it. That mattered beyond a missing feature: ADR-034's relevance curves were fitted at document granularity, so the app page could not request the retrieval shape its own relevance numbers were calibrated on. Brings the endpoint into line with its sibling on all four points: the value is read, an unknown value is rejected rather than silently downgraded to chunk, the document+semantic combination is refused with the same 422 payload, and the value reaches both the single-search and doc_types branches of the algorithm call. Search metrics now report the granularity actually used. Found by auditing which search settings the Astrolabe UI can reach versus what the server supports, after a retrieval benchmark turned out to be sweeping parameters no deployment could request. Deck #1070. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude · 2026-08-19T20:46:14Z

Claude finished @cbcoutinho's task in 2m 26s —— View job

Review

Gather context (PR body, changed files)
Read nextcloud_mcp_server/api/visualization.py diff
Read new test file
Check sibling endpoint (/api/v1/search) for parity comparison
Post review feedback

Overall this is a clean, well-motivated parity fix — the new validation/422/prefetch logic in vector_search mirrors unified_search closely (granularity read, unknown-value rejection, document+semantic 422, granularity threaded onto both the single-search and doc_types branches, and grouped= now driven by the requested granularity). Test coverage of the happy paths is thorough (test_vector_viz_granularity_api.py).

One real bug found in the diff, plus a note on the test-coverage gap that would have caught it.

🔴 Bug: `UnboundLocalError` in the error-path metric

In nextcloud_mcp_server/api/visualization.py, vector_search pre-binds only algorithm/fusion before the try block:

# line 873-874
algorithm = "unknown"
fusion = "rrf"

But granularity is not pre-bound — it's first assigned at line 904, inside the try, after body = await request.json() and the min_relevance parsing. The except Exception handler at line 1208 now references it unconditionally:

# line 1216-1222
record_search_request(
    surface="http_viz",
    algorithm=_search_algorithm_label(algorithm, fusion),
    granularity=granularity,   # <-- not guaranteed to be bound
    reranked="false",
    status="error",
)

If anything before line 904 raises — e.g. malformed JSON (await request.json()), or a non-numeric limit (min(body.get("limit", 10), 50) at line 883 raises TypeError if limit is a string/list) — the except block itself raises UnboundLocalError while trying to read granularity. That replaces the intended sanitized 500 response (_sanitize_error_for_client) with an unhandled crash from inside the exception handler, and the logger.exception("Error in vector search") call for the original error still runs, but the response you actually wanted to return never gets built.

The sibling unified_search avoids exactly this by pre-binding all three at the top (line 399-401):

algorithm = "unknown"
granularity = GRANULARITY_CHUNK
fusion = "rrf"

Fix: add granularity = GRANULARITY_CHUNK next to the existing algorithm/fusion pre-binding at line 873-874 in vector_search.

Fix this →

🟡 Test coverage gap

test_vector_viz_granularity_api.py covers the four parity points well but doesn't exercise the error path (e.g. a malformed body / non-numeric limit) — which is exactly the scenario that trips the bug above. Worth adding a case asserting the endpoint still returns a sanitized 500 (not an unhandled exception) when parsing fails before granularity is resolved, alongside the pre-binding fix.

🟢 Nit

Line 475 formats the invalid-granularity error with list(VALID_GRANULARITIES) while the new code at line 910 uses sorted(VALID_GRANULARITIES). Both currently produce the same output since VALID_GRANULARITIES = ("chunk", "document") is already alphabetical, so this is cosmetic only — not blocking.

sonarqubecloud · 2026-08-19T20:46:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Navigation Menu

feat(api): accept `granularity` on /api/v1/vector-viz/search - #1359

feat(api): accept `granularity` on /api/v1/vector-viz/search#1359
cbcoutinho wants to merge 1 commit into
fix/rerank-warning-false-positivefrom
feat/search-setting-parity

cbcoutinho commented Aug 19, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 19, 2026 •

edited

Loading

Uh oh!

sonarqubecloud Bot commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

cbcoutinho commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How it was found

What changed

Compatibility

Test coverage

Uh oh!

claude Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

🔴 Bug: UnboundLocalError in the error-path metric

🟡 Test coverage gap

🟢 Nit

Uh oh!

sonarqubecloud Bot commented Aug 19, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

cbcoutinho commented Aug 19, 2026 •

edited

Loading

claude Bot commented Aug 19, 2026 •

edited

Loading

🔴 Bug: `UnboundLocalError` in the error-path metric